Skip to content

kv-cache : remove LLAMA_SET_ROWS checks #15505

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Aug 28, 2025
Merged

kv-cache : remove LLAMA_SET_ROWS checks #15505

merged 1 commit into from
Aug 28, 2025

Conversation

@ggerganov
Copy link
Member

target #15504

With the adoption of ggml_set_rows() across all backends (#14661) we can now remove the old fallback path and simplify the implementation.

@github-actions github-actions bot added ggml changes relating to the ggml tensor library for machine learning Ascend NPU issues specific to Ascend NPUs labels Aug 22, 2025
@ggerganov
Copy link
Member Author

The only backends that currently are going to be affected by this change are CANN and OpenCL when using quantized KV cache. With standard F16 KV cache there should be no problems.

@hipudding @max-krasnyansky @lhez FYI

Base automatically changed from gg/kv-cache-reuse-layers to master August 24, 2025 10:07
@ggerganov ggerganov force-pushed the gg/kv-cache-remove-set-rows-checks branch from 44933f7 to 642d79e Compare August 24, 2025 10:07
@ggerganov ggerganov force-pushed the gg/kv-cache-remove-set-rows-checks branch from 642d79e to 61b7fe2 Compare August 27, 2025 10:57
@ggerganov ggerganov mentioned this pull request Aug 27, 2025
Open
4 tasks
@ggerganov ggerganov merged commit 8a4280c into master Aug 28, 2025
59 of 60 checks passed
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request Aug 28, 2025
@gabe-l-hart
Merge remote-tracking branch 'origin/master' into gabe-l-hart/nvidia-…
05a5ce3 
…nemotron-nano-15409

* origin/master: (59 commits)
scripts: add sqlite3 check for compare-commits.sh (ggml-org#15633)
kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505)
gguf-py: byteswapping improvements (ggml-org#12851)
cli : change log to warning to explain reason for stopping (ggml-org#15604)
model-conversion : add mmproj conversion target (ggml-org#15628)
cuda: Add cublasLt_static linking when GGML_STATIC is enabled (ggml-org#15622)
server: higher timeout for tests (ggml-org#15621)
presets : add qwen3-30B-a3b FIM (ggml-org#15616)
HIP: Enable support for ggml_backend_cuda_register_host_buffer (ggml-org#15615)
kv-cache : better estimate of n_kv for multi-sequence batches (ggml-org#15610)
CANN: refactor mask handling and improve performance in FA (ggml-org#15561)
ggml-cpu : add basic RVV support for vector f32 ops (ggml-org#15057)
common : add -m to bash completion for --model [no ci] (ggml-org#15591)
OpenCL: add fused group_norm/norm, mul, add (ggml-org#15314)
tests : fix test-opt with GGML_BACKEND_DL (ggml-org#15599)
SYCL: fix rms_norm_mul_add for tensor dim not a multiple of sg_size (ggml-org#15592)
mtmd : fix mtmd ios build (ggml-org#15579)
tests: add performance test for mul mat id (ggml-org#15543)
llamafile: PowerPC Sgemm Optimization (ggml-org#15558)
graph : fix assert in memory-less build_attn (ggml-org#15590)
...
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request Aug 28, 2025
@gabe-l-hart
Merge remote-tracking branch 'origin/master' into gabe-l-hart/alora-s…
37d0bd0 
…upport

* origin/master: (61 commits)
scripts: add sqlite3 check for compare-commits.sh (ggml-org#15633)
kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505)
gguf-py: byteswapping improvements (ggml-org#12851)
cli : change log to warning to explain reason for stopping (ggml-org#15604)
model-conversion : add mmproj conversion target (ggml-org#15628)
cuda: Add cublasLt_static linking when GGML_STATIC is enabled (ggml-org#15622)
server: higher timeout for tests (ggml-org#15621)
presets : add qwen3-30B-a3b FIM (ggml-org#15616)
HIP: Enable support for ggml_backend_cuda_register_host_buffer (ggml-org#15615)
kv-cache : better estimate of n_kv for multi-sequence batches (ggml-org#15610)
CANN: refactor mask handling and improve performance in FA (ggml-org#15561)
ggml-cpu : add basic RVV support for vector f32 ops (ggml-org#15057)
common : add -m to bash completion for --model [no ci] (ggml-org#15591)
OpenCL: add fused group_norm/norm, mul, add (ggml-org#15314)
tests : fix test-opt with GGML_BACKEND_DL (ggml-org#15599)
SYCL: fix rms_norm_mul_add for tensor dim not a multiple of sg_size (ggml-org#15592)
mtmd : fix mtmd ios build (ggml-org#15579)
tests: add performance test for mul mat id (ggml-org#15543)
llamafile: PowerPC Sgemm Optimization (ggml-org#15558)
graph : fix assert in memory-less build_attn (ggml-org#15590)
...
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request Aug 28, 2025
@gabe-l-hart
Merge remote-tracking branch 'origin/master' into gabe-l-hart/thinkin…
e2a19d2 
…g-model-disabled-agent-prefill

* origin/master: (76 commits)
scripts: add sqlite3 check for compare-commits.sh (ggml-org#15633)
kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505)
gguf-py: byteswapping improvements (ggml-org#12851)
cli : change log to warning to explain reason for stopping (ggml-org#15604)
model-conversion : add mmproj conversion target (ggml-org#15628)
cuda: Add cublasLt_static linking when GGML_STATIC is enabled (ggml-org#15622)
server: higher timeout for tests (ggml-org#15621)
presets : add qwen3-30B-a3b FIM (ggml-org#15616)
HIP: Enable support for ggml_backend_cuda_register_host_buffer (ggml-org#15615)
kv-cache : better estimate of n_kv for multi-sequence batches (ggml-org#15610)
CANN: refactor mask handling and improve performance in FA (ggml-org#15561)
ggml-cpu : add basic RVV support for vector f32 ops (ggml-org#15057)
common : add -m to bash completion for --model [no ci] (ggml-org#15591)
OpenCL: add fused group_norm/norm, mul, add (ggml-org#15314)
tests : fix test-opt with GGML_BACKEND_DL (ggml-org#15599)
SYCL: fix rms_norm_mul_add for tensor dim not a multiple of sg_size (ggml-org#15592)
mtmd : fix mtmd ios build (ggml-org#15579)
tests: add performance test for mul mat id (ggml-org#15543)
llamafile: PowerPC Sgemm Optimization (ggml-org#15558)
graph : fix assert in memory-less build_attn (ggml-org#15590)
...
ggerganov
ggerganov commented Aug 28, 2025
View reviewed changes
src/llama-kv-cache.cpp

// only find a suitable slot for the ubatch. don't modify the cells yet
const auto sinfo_new = find_slot(ubatch, cont);
const auto sinfo_new = find_slot(ubatch, true);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should have been false - fixing ..

@ggerganov ggerganov mentioned this pull request Aug 28, 2025
Merged
@ggerganov ggerganov deleted the gg/kv-cache-remove-set-rows-checks branch August 28, 2025 14:07
gabe-l-hart added a commit to gabe-l-hart/llama.cpp that referenced this pull request Aug 28, 2025
@gabe-l-hart
Merge remote-tracking branch 'origin/master' into gabe-l-hart/nvidia-…
cb03b4f 
…nemotron-nano-15409

* origin/master: (59 commits)
scripts: add sqlite3 check for compare-commits.sh (ggml-org#15633)
kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505)
gguf-py: byteswapping improvements (ggml-org#12851)
cli : change log to warning to explain reason for stopping (ggml-org#15604)
model-conversion : add mmproj conversion target (ggml-org#15628)
cuda: Add cublasLt_static linking when GGML_STATIC is enabled (ggml-org#15622)
server: higher timeout for tests (ggml-org#15621)
presets : add qwen3-30B-a3b FIM (ggml-org#15616)
HIP: Enable support for ggml_backend_cuda_register_host_buffer (ggml-org#15615)
kv-cache : better estimate of n_kv for multi-sequence batches (ggml-org#15610)
CANN: refactor mask handling and improve performance in FA (ggml-org#15561)
ggml-cpu : add basic RVV support for vector f32 ops (ggml-org#15057)
common : add -m to bash completion for --model [no ci] (ggml-org#15591)
OpenCL: add fused group_norm/norm, mul, add (ggml-org#15314)
tests : fix test-opt with GGML_BACKEND_DL (ggml-org#15599)
SYCL: fix rms_norm_mul_add for tensor dim not a multiple of sg_size (ggml-org#15592)
mtmd : fix mtmd ios build (ggml-org#15579)
tests: add performance test for mul mat id (ggml-org#15543)
llamafile: PowerPC Sgemm Optimization (ggml-org#15558)
graph : fix assert in memory-less build_attn (ggml-org#15590)
...
Minh141120 pushed a commit to janhq/llama.cpp that referenced this pull request Aug 29, 2025
@ggerganov @Minh141120
kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505)
246385a 
ggml-ci
graehl added a commit to graehl/llama.cpp that referenced this pull request Sep 18, 2025
@graehl
increase graph_max_nodes for finetune
5ef321d 
fix regression during finetune on Llama-3.2-1B-F32:
GGML_ASSERT(cgraph->n_nodes < cgraph->size) failed

git bisect applying the most recent finetune (SGD) change
showed that
d498af3 Georgi Gerganov 2025-07-18 14:31:15 +0300 graph : avoid huge
warm-up graphs for MoE models (ggml-org#14753)

which greatly decreased graph_max_nodes
has been responsible for finetune failing on reasonably sized models
for the past two months. partially reverting the decrease (maybe
larger models still fail)

note: env LLAMA_SET_ROWS=0 is needed also or else:
GML_ASSERT(!node->view_src || node->op == GGML_OP_CPY || node->op == GGML_OP_VIEW || node->op == GGML_OP_RESHAPE || node->op == GGML_OP_PERMUTE || node->op == GGML_OP_TRANSPOSE) failed
(the node->op in question is indeed a rows op)
unfortunately a git revert on:

8a4280c Georgi Gerganov 2025-08-28 12:27:02 +0300 kv-cache : remove
LLAMA_SET_ROWS checks (ggml-org#15505)

is not straightforward, so this branch is behind that.
@graehl graehl mentioned this pull request Sep 18, 2025
Open
graehl added a commit to graehl/llama.cpp that referenced this pull request Sep 18, 2025
@graehl
increase graph_max_nodes for finetune
4add018 
fix regression during finetune on Llama-3.2-1B-F32:
GGML_ASSERT(cgraph->n_nodes < cgraph->size) failed

git bisect applying the most recent finetune (SGD) change
showed that
d498af3 Georgi Gerganov 2025-07-18 14:31:15 +0300 graph : avoid huge
warm-up graphs for MoE models (ggml-org#14753)

which greatly decreased graph_max_nodes
has been responsible for finetune failing on reasonably sized models
for the past two months. partially reverting the decrease (maybe
larger models still fail)

note: env LLAMA_SET_ROWS=0 is needed also or else:
GML_ASSERT(!node->view_src || node->op == GGML_OP_CPY || node->op == GGML_OP_VIEW || node->op == GGML_OP_RESHAPE || node->op == GGML_OP_PERMUTE || node->op == GGML_OP_TRANSPOSE) failed
(the node->op in question is indeed a rows op)
unfortunately a git revert on:

8a4280c Georgi Gerganov 2025-08-28 12:27:02 +0300 kv-cache : remove
LLAMA_SET_ROWS checks (ggml-org#15505)

is not straightforward, so this branch is behind that.
Nexesenex added a commit to Nexesenex/croco.cpp that referenced this pull request Oct 6, 2025
@Nexesenex
Revert "kv-cache : remove LLAMA_SET_ROWS checks (ggml-org#15505) ggml…
db193bf 
…-ci"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danbev danbev danbev approved these changes

Assignees

No one assigned

Labels

Ascend NPU issues specific to Ascend NPUs ggml changes relating to the ggml tensor library for machine learning

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@ggerganov @danbev